-
Notifications
You must be signed in to change notification settings - Fork 192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resolve #389: Add line numbers to recog_verify output #390
Conversation
Output before this patch:
Output after this patch:
|
Realized we have some tests here that are looking at literal log output, so I need to update the patch to fix those as well. |
Nokogiri under jruby apparently has compatibility problems that cause it to have line numbers that are off by one: Tabling for now |
It would be nice to simplify the reporting with a simple inline line number.
|
Per the comment from @dabdine above it looks like this may be fixed in https://github.com/sparklemotion/nokogiri/releases/tag/v1.11.2 As a note, here is the version that was used in the Edit: Correction, it appears the |
New patch updates the logging format (suggestion from @mkienow-r7):
|
Still haven't addressed the jruby issue, so expecting the build to fail due to the off-by-one problem Jruby has when "approximating" the line from the XML document. |
Ah I think I know what the issue is here. I think this is due to the line numbering logic in Nokogiri under JRuby ignoring the xml prolog tag:
EDIT: Yep, removing the XML tag from the test XML files makes this build pass. So, this is an issue with upstream Nokogiri that needs to be addressed. |
a0c5b82
to
b465f05
Compare
Opened an issue for this w/Nokogiri. I added a test on their end too to demonstrate the issue. However, the Java source code seems a bit more complicated to manipulate -- I believe the entire DOM parser used would need to be rewritten. For now, I propose disabling these tests when in JRuby. Here's the issue for tracking purposes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This enhancement will be very helpful when tracing errors and warnings back to their source. Thank you for the contribution @dabdine!
Description
Just a simple patch to produce line numbers in parsed fingerprints.
Motivation and Context
Need this to tell which fingerprints are which when diagnosing and fixing problems reported by
recog_verify
How Has This Been Tested?
Ran
recog_verify
Ran
bundle exec rake tests
Types of changes
Checklist: